Add native support for WASM targets #14
Conversation
esimkowitz
changed the title
esimkowitz
changed the title
|
Hi, Thanks for the PR, honestly the sys part looks good, however the testing does not:
I'd rather prefer a pre-processor which automatically modifies the code when compiling to wasm instead of manually adding these non-standard attributes to the code. Perhaps such a tool could be made as from what I've seen you don't call "cargo test" to run these tests anyway? That said, I'm ok merging the system.rs part, as it looks straight forward and might be a nice addition. Would this be enough for your use case? |
|
Yeah, the situation for testing WASM targets is a bit messy, the existing test frameworks I found all required their own attributes to be added to the test methods. I can certainly look into swapping the attributes in a preprocessor but if you're fine just checking in the system.rs change for now, I can update the branch and contribute the preprocessor as a separate change. |
|
Thanks for the latest changes, I'll merge this probably tomorrow as the computer I use right now does not have the publish key. |
This PR updates the DateTime converter implementation to use the [time-tz crate](https://github.com/Yuri6037/time-tz) instead of chrono-tz. This patches https://github.com/esimkowitz/dev-widgets/security/dependabot/1. It also adds functionality to get the local timezone for the target system. The specific revision of this crate used here includes a proposed change I have added which extends support for getting the local time zone to WASM targets (Yuri6037/time-tz#14). This PR also adds support for logging, replacing use of println! in the application logic. This way, support for logging will be equal for both WASM and desktop targets. It also removes an unnecessary direct clap dependency.
|
This is now available in v2.0.0. I had to bump major due to the change of error enum (new unsupported variant). |
Summary
This PR adds support for getting the configured system time zone via the js-sys crate for WASM targets. It also updates the dependencies to include inherent support for WASM targets by including the
wasm-bindgenfeature from the time crate. It also adds necessary attributes and dependencies for running all the existing tests against WASM targets.I have a working implementation of this change here.
Reason for the change
I am building a cross-platform application that is a collection of useful widgets for developers. One of the features I currently support is converting a Unix timestamp to different locales. Previously, I was using the chrono-tz crate to get the IANA database, but I am working to adopt this crate instead as it patches GHSA-wcg3-cvx6-7396.
I was able to migrate my DateTime converter widget to use your crate easily, it only required patching the time crate to add the wasm-bindgen feature for WASM targets. I also wanted to add the system time zone feature, though, since that was not supported at all in chrono-tz, but it was failing for WASM targets since they don't have access to the Unix or Windows TZ syscalls.
Implementation details
The specific approach I took is based on https://stackoverflow.com/questions/61654015/how-can-i-get-the-browsers-local-timezone-using-rust-js-sys. It involves using the js-sys crate to invoke the Intl.DateTimeFormat object, which in its default form returns the time zone locale as configured in the browser/system runtime. Helpfully, the timeZone field is already formatted per the IANA standards, meaning it can easily be parsed by
get_by_name, the same as the other targets.Documentation
I've updated the system module with more verbose documentation and validated it using
cargo doc. I've also added a new errorUnsupported, which will be returned ifget_timezoneis called for an unsupported target platform (not WASM, Unix, or Windows).Testing
I added support for testing against WASM browser targets using wasm-bindgen-test. This does not impact testing or building against other targets as the attribute is restricted to targets in the WASM family. Running these tests requires either the wasm-pack tool or the wasm-bindgen-cli be installed, otherwise the WASM binary cannot be run.
I've also updated the CI pipeline to add a new job that builds the crate against the
wasm32-unknown-unknowntarget and runs all tests against Chrome, Firefox, and Safari.All tests are passing:
➜ time-tz git:(evan/js-sys) ✗ cargo test --target wasm32-unknown-unknown --all-features Compiling time-tz v1.0.3 (/Users/esimk/source/time-tz) Finished test [unoptimized + debuginfo] target(s) in 3.19s Running unittests src/lib.rs (target/wasm32-unknown-unknown/debug/deps/time_tz-6ba78c3ee7233970.wasm) Set timeout to 20 seconds... running 12 tests test time_tz::posix_tz::parser::tests::basic ... ok test time_tz::tests::handles_backward_changeover ... ok test time_tz::tests::handles_broken_time ... ok test time_tz::tests::handles_after_changeover ... ok test time_tz::tests::handles_forward_changeover ... ok test time_tz::tests::dst ... ok test time_tz::tests::london_to_berlin ... ok test time_tz::tests::offsets_and_name ... ok test time_tz::tests::find ... ok test time_tz::tests::names ... ok test time_tz::system::tests::get_timezone ... ok test time_tz::binary_search::tests::test_binary_search ... ok test result: ok. 12 passed; 0 failed; 0 ignoredNon-Goals
This change has only been validated against the
wasm32-unknown-unknowntarget, as thewasm32-wasitarget is still experimental and likely to change.wasm32-unknown-emscriptenis generally viewed as legacy so I also have not validated against this target.I have not added support for running tests against Node as my primary goal here was to support browser compatibility, since I compile natively for Windows/Unix targets. The wasm-bindgen-test crate does not provide good support for targeting tests at both Node and browsers as each test module needs to call the
wasm_bindgen_test_configuremacro to set up browser compatibility. I thought about exposing the choice to target testing at either browser or Node as a feature flag, but this would interfere with the ability to run test with the --all-features option.